-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Modules] Don't const eval VarDecls with dependent type #147378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Henrik G. Olsson (hnrklssn) ChangesEvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in #145447 Full diff: https://github.com/llvm/llvm-project/pull/147378.diff 2 Files Affected:
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e0362245d6ecd..f25e9fe7e8f2f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2450,7 +2450,7 @@ bool VarDecl::hasInitWithSideEffects() const {
ES->HasSideEffects =
E->HasSideEffects(getASTContext()) &&
// We can get a value-dependent initializer during error recovery.
- (E->isValueDependent() || !evaluateValue());
+ (E->isValueDependent() || getType()->isDependentType() || !evaluateValue());
ES->CheckedForSideEffects = true;
}
return ES->HasSideEffects;
diff --git a/clang/test/Modules/var-init-side-effects-templated.cpp b/clang/test/Modules/var-init-side-effects-templated.cpp
new file mode 100644
index 0000000000000..542ca270429b6
--- /dev/null
+++ b/clang/test/Modules/var-init-side-effects-templated.cpp
@@ -0,0 +1,20 @@
+// Tests referencing variable with initializer containing side effect across module boundary
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -o %t
+
+export module Foo;
+
+export template <class Float>
+struct Wrapper {
+ double value;
+};
+
+export constexpr Wrapper<double> Compute() {
+ return Wrapper<double>{1.0};
+}
+
+export template <typename Float>
+Wrapper<Float> ComputeInFloat() {
+ const Wrapper<Float> a = Compute();
+ return a;
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well.
793b129 to
6c494ad
Compare
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in llvm#145447 (cherry picked from commit c885005)
|
Thanks for the quick fix @hnrklssn !! |
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in llvm#145447
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in llvm#145447
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in llvm#145447 (cherry picked from commit c885005)
EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in #145447